Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: initial intent API implementation #453

Closed

Conversation

jonaslagoni
Copy link
Member

@jonaslagoni jonaslagoni commented Jan 21, 2022

This is the first part of the intent API implementation.

I have only implemented the root IntentAsyncAPIDocument as I thought this was a great time to allow other contributors to help out if they want to, by creating first good issues. Even though we might need to place a time constraint on when they need to be done (in order to have enough time to evaluate the intents before releasing).

Questions and Decisions

  1. How should we interact with the raw AsyncAPI document?
    • Currently, I have decided to stay with our current model and reuse that. This also means that users can have access to them if that is what they desire. Instead of just raw JSON. This can be done through intent.rawDocument().
  2. How should we define operation types?
    • Currently, I have decided to use the following OperationTypes, that forced me to change the Operation.type() -> Operation.types()
  3. How much of the old intents and model utils should we keep (anonymousNaming, iterators, customValidators)?
    • Currently, I have kept everything as is and just moved them around. See below.

What changed

Todo:

  • Change merge into branch to ???
  • Make tests for IntentAsyncAPIDocument

Todo once merged:

  • Figure out what to do with message conversion for custom schema formats
  • Based on the discussion 2, change the Operation.type() -> Operation.types() in parser-api repo.
  • Create good first issues for the rest of the implementation to do as much parallel development as possible.
  • Port the implementation to TS
  • Create pre-release generator to start utilizing it right away

Related issue(s)

Fixes #401

@smoya smoya force-pushed the feature/mock_implementation branch from 6cb2fac to 4481bcf Compare February 21, 2022 11:24
@smoya
Copy link
Member

smoya commented Feb 21, 2022

I added a new commit with the implementation for the suggestions @fmvilas made here.
I ended up creating an intermediate object for channels, operations, and messages.

cc @jonaslagoni

@smoya
Copy link
Member

smoya commented Feb 22, 2022

I think we would also need to introduce methods for getting objects by their ID, like document.channel(<channel_id>), document.operation(<operation_id>), document.server(<server_name>).

Edit: this is covered by document.channels().filterById(), document.operation().filterById(), document.messages().filterById(), document.servers().filterById()

@smoya
Copy link
Member

smoya commented Feb 22, 2022

  • Added IntentServers object with same style as IntentChannels, IntentOperations, and ` IntentMessages).
  • Added a id() method on the following models:
    • Channel -> returns key on the map for 2.x.x
    • Operation -> returns operation id for 2.x.x
    • Message -> returns uid for 2.x.x
    • Server -> returns key on the map for 2.x.x
  • Removed document.channel(<channel_name>) method in favor of document.channels().filterById(<name>).

@smoya smoya force-pushed the feature/mock_implementation branch from 7f2bde3 to 3ba6710 Compare February 22, 2022 13:40
@smoya
Copy link
Member

smoya commented Feb 22, 2022

I'm thinking about introducing an isEmpty() method to all those intermediate objects, so we can remove the document.hasChannels-like methods.
The point is that isEmpty is the opposite of what we previously had (emptiness vs fullness) so not sure what others think about.

@fmvilas
Copy link
Member

fmvilas commented Feb 25, 2022

I'm thinking about introducing an isEmpty() method to all those intermediate objects, so we can remove the document.hasChannels-like methods. The point is that isEmpty is the opposite of what we previously had (emptiness vs fullness) so not sure what others think about.

I think it's a good idea 👍

Operation -> returns operation id for 2.x.x

An operation might not contain an operationId, so it's possible we get undefined. However, in v3, the "operationId" is mandatory (it's implicit), so I wonder if we shouldn't be generating operation ids for those operations that don't have one defined. This way we can make sure we never return undefined in any version.

@magicmatatjahu magicmatatjahu mentioned this pull request Mar 3, 2022
20 tasks
@sonarcloud
Copy link

sonarcloud bot commented Mar 7, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@smoya
Copy link
Member

smoya commented Mar 7, 2022

I wonder if we shouldn't be generating operation ids for those operations that don't have one defined. This way we can make sure we never return undefined in any version.

Yeah, I think it makes sense. Would you also suggest we write it as a rule for all parser implementations (Parser-API) ? So the return type should be only string and never empty then.

@fmvilas
Copy link
Member

fmvilas commented Mar 8, 2022

Would you also suggest we write it as a rule for all parser implementations (Parser-API) ?

I think so. Not sure how far we should get into implementation details other than the API surface but definitely something to take into account. In any case, if we say that operations MUST always have an ID, implementations will have to generate them 😄

@smoya
Copy link
Member

smoya commented Mar 8, 2022

@jonaslagoni would you mind converting this issue into a draft? As this is just a POC, I don't think it makes sense to keep it as a regular PR.

@github-actions
Copy link

github-actions bot commented Jul 7, 2022

This pull request has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation.

There can be many reasons why some specific pull request has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this pull request forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Jul 7, 2022
@smoya
Copy link
Member

smoya commented Jul 7, 2022

@jonaslagoni We can close this one as all this work is being done on the next-major branch.

@jonaslagoni jonaslagoni closed this Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Parser-API (Intent-Driven)
4 participants